Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: rework is_bipartite to do an actual bipartivity check #1729

Closed
wants to merge 3 commits into from
Closed

feat: rework is_bipartite to do an actual bipartivity check #1729

wants to merge 3 commits into from

Conversation

schochastics
Copy link
Contributor

Fix #709
A suggestion how a reworked is_bipartite could look like that does not (or should not) break existing code.

library(igraph)

#bipartite no type
is_bipartite(make_ring(10))
#> Warning: `graph` is bipartite but has no 'type' vertex attribute. You can add it using:
#> `V(graph)$type <- bipartite_mapping(graph)$type`
#> [1] TRUE

# not bipartite but type
g <- make_full_graph(4)
V(g)$type <- c(TRUE,TRUE,FALSE,FALSE)
is_bipartite(g)
#> Warning: `graph` has a type attribute but is not actually bipartite.
#> [1] FALSE

#bipartite and type
g <- make_ring(4)
V(g)$type <- c(TRUE,FALSE,TRUE,FALSE)
is_bipartite(g)
#> [1] TRUE

#bipartite wrong type
g <- make_ring(4)
V(g)$type <- c(TRUE,TRUE,FALSE,FALSE)
is_bipartite(g)
#> Warning: `graph` is bipartite but has a wrong 'type' vertex attribute. You can correct
#> it using: `V(graph)$type <- bipartite_mapping(graph)$type`
#> [1] TRUE

Created on 2025-02-28 with reprex v2.1.1

@szhorvat, thoughts?

@aviator-app Aviator app
Copy link
Contributor

aviator-app bot commented Feb 28, 2025 •
edited
Loading

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR is currently in state closed (PR closed manually).

See the real-time status of this PR on the Aviator webapp.
Use the Aviator Chrome Extension to see the status of your PR within GitHub.

@schochastics schochastics marked this pull request as draft February 28, 2025 19:40
@szhorvat
Copy link
Member

szhorvat commented Mar 4, 2025

Let me check I understand the proposal:

  • is_bipartite() would return whether the graph is mathematically bipartite, regardless of any type attribute. This is actually a breaking change.
  • If the presence/absence and value of the type attribute is not consistent with whether the graph is mathematically bipartite, a warning is issued. A warning does not normally abort running code.

Some things that I am not 100% comfortable with at this point:

  • We have three things to think about: (1) Are the vertices marked as belonging to two categories? (2) If yes, do edges run only across categories? (3) Is it in principle possible to assign categories so that edges run only across categories, not within categories (mathematically bipartite)? Arguably, all three are useful. We had a function to test for (1), which is now changed to test for (3), but we don't have any way to test for (2). Something to think about is whether it is useful to operate with two-category networks where some edges run within categories. For example, it is perfectly possible to compute a bipartite projection even if there are within-category edges—this isn't currently allowed but it could be.
  • Non-connected graphs have more than one valid two-colouring. Thus the message about just adding bipartite_mapping(graph)$type is not entirely sound advice. The check implemented seems to verify if the type attribute matches bipartite_mapping(graph)$type, but this is incorrect. Consider this network: 1-2-3, 4-5. If 1 and 3 are blue, 2 is red, then 4-red, 5-blue and 4-blue 5-red are both valid assignments.

My gut feeling is that it'd be nice to have different nomenclature for (1) and (3) and a separate function to check for (2). It'd be best to look at case studies to find what's the most useful (and whether breaking changes are worth it).

@schochastics
Copy link
Contributor Author

Thanks for your thoughtful reply! This makes a lot of sense and I think the topic does need more thinking about. I will close this PR for now and reopen eventually when there is a better idea on how to implement this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

is_bipartite() is confusing / problematic
2 participants
@schochastics @szhorvat